Skip to content

Reject directory destinations in OSFS.copy - #613

Open
Aathi-27 wants to merge 3 commits into
PyFilesystem:masterfrom
Aathi-27:fix-osfs-directory-destination
Open

Aathi-27 wants to merge 3 commits into
PyFilesystem:masterfrom
Aathi-27:fix-osfs-directory-destination

Conversation

@Aathi-27

Copy link
Copy Markdown

Type of changes

  • Bug fix
  • Tests

Description

Fixes #496.

Problem

OSFS.copy(..., overwrite=True) can treat a directory destination as a container. The same-instance copy_file path can then overwrite a same-named file inside that directory instead of rejecting the destination.

Fix

Reject directory destinations in _check_copy with FileExpected, before either copy implementation runs. Existing source checks, DestinationExists when overwrite is disabled, and same-path checks keep their current order.

Tests

  • Added regressions for empty and occupied directories, same-instance copy_file, directory symlinks, root destinations and error precedence. The new tests fail on the unchanged source and pass with the fix.
  • OSFS/copy suite: 141 passed, 2 skipped.
  • Non-FTP suite: 2,399 passed, 23 skipped.
  • Full suite: 2,679 passed, 26 skipped, 3 FTP teardown errors. Unchanged baseline: 2,654 passed, 26 skipped, the same 3 teardown errors from pyftpdlib calling Process.net_connections with psutil 5.9.8.
  • Repository-wide pinned flake8, latest Black on changed Python files, and git diff --check pass.

Limitations

Tested on Linux with Python 3.10. Legacy sendfile/fallback method source was checked on that runtime, not on an older interpreter. Windows and macOS were not tested. This validation check does not make copying atomic or protect against external filesystem changes after validation.

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @PyFilesystem/maintainers may be pedantic in the code review.

@Aathi-27

Copy link
Copy Markdown
Author

Could a maintainer please review the fix for #496 and approve the pending GitHub Actions run? AppVeyor is passing. Thanks.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.copy.copy_file: Breaking changes between Python 3.7 and Python 3.8

1 participant